-
Notifications
You must be signed in to change notification settings - Fork 3.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Optimize GlobeSurfaceShaderSet.getShaderProgram #2346
Conversation
The steady-state speedup is nice, but this increases tile load time a bit and it makes tiles much larger. I think I can fix both of those without impacting the steady-state performance. I'm working on it now. |
Sounds good, thanks. Why does it increase load time? Web worker transfer? As for tile size, my only concern would be L2 cache pollution (I actually think I am running into this elsewhere for setting uniforms) given the tile itself is tiny compared to the renderer resources. |
Because the first time we see a tile, we're now building up a giant shader string and then looking that up in the shader cache. Previously we built a smaller key and looked that up. It should be possible to avoid string lookups entirely.
Yeah, exactly. We end up looking at a lot more tiles during tile selection than we actually end up rendering. I don't know if the size increase ends up having a measurable impact on the performance due to cache pollution, but as long as we're optimizing this up we might as well optimize it a bit better, since it's not much harder. |
Ah, I almost keep the older cache too, but a non-string lookup sounds good. As for L2, I've thought about - in general - organizing objects so that the properties needed for traversal are tightly defined with a pointer to another object with all the other properties. I believe this is a common old-school game technique for fitting as many objects as possible into cache lines. I haven't tried this because we have bigger fish to fry, but keeping the tile tight for now sounds good. Just pack them into an int or two or whatever. |
Ok this should be in good shape now. New tiles can now get an existing shader without any string concatenations or comparisons. |
Someone else should probably merge this now that I've changed it pretty significantly. |
All looks good, thanks! By the way, is it possible to use the squared distance instead for |
Optimize GlobeSurfaceShaderSet.getShaderProgram
Yes, almost certainly. Actually the entire |
Want to add it to the roadmp (#526) - or just do it! 👍 |
For 3K fullscreen view of Mount Everest, it was
2.3%
to2.5%
of the profile (one of the top hot-spots).Now it is
~0.07%
.@kring at this point, we might want to rename
GlobeSurfaceShaderSet
to something else and make it just a function.